[H1410101] iP #555

H1410101 · 2023-09-04T12:53:08Z

For class reasons; ignore.

CatBot

You might be wondering, why would you use CatBot?

it has cats

There is no better reason, and there needs no other reason.

It features peak programmer syntax, like this amazing, lovable ~~monstrosity~~ little guy.

Here's a snippet for your convenience:

BiConsumer<String, BiFunction<
                NamedParameterMap, BiConsumer<ErrorIndicatorIo.InvalidArgumentState, NamedParameterMap>,
                Optional<Task>>>
                createTaskIfValidElseWarn = (args, bifunction) -> slashPattern.ifParsableElseDefault(args,
                        namedParameterMap -> bifunction.apply(
                                namedParameterMap,
                                io::indicateArgumentInvalid
                        ).ifPresent(task -> {
                            taskList.addTask(task);
                            io.displayTaskAdded(taskList);
                        })
                );

        commands.addCommand("todo",
                        args -> createTaskIfValidElseWarn.accept(args, Task.Todo::createIfValidElse)
                )
                .addCommand("event",
                        args -> createTaskIfValidElseWarn.accept(args, Task.Event::createIfValidElse)
                )
                .addCommand("deadline",
                        args -> createTaskIfValidElseWarn.accept(args, Task.Deadline::createIfValidElse)
            );

That's right! Your 👀 do not deceive you. That is a

BiConsumer that takes in a
BiFunction, which takes in a
BiConsumer as one of its arguments
AND returns an Optional!

I bet you've never seen that many angle brackets in a row since CS2030S.

Programs must be written for machines to execute, and only incidentally for people to partially comprehend in flashes of brilliance after large swathes of agony.
~ Me, 2023, while descending into the infinite abyss, 2 weeks behind schedule

…rguments

…rgument errors

rayyan35p

Good code quality mostly, just need to add header comments and rename variables to make it clear. 💯

rayyan35p · 2023-09-06T16:01:34Z

src/main/java/catbot/CatBot.java

+    private static void addSupportedCommandsToCommandMap() {
+        CommandPattern<Integer> integerPattern = CatBotCommandPatterns.getIntegerPatternGenerator()
+                .generate(io::indicateInvalidInteger);
+        CommandPattern<NamedParameterMap> slashPattern = CatBotCommandPatterns.getSlashPatternGenerator()
+                .generate(CatBotCommandPatterns.NO_DEFAULT);
+
+        commands.setDefaultCommand(io::indicateInvalidCommand)
+                .addCommand("bye", args -> io.cleanup())
+                .addCommand("list", args -> io.printTaskList(taskList));
+
+        // User modifying existing tasks (through IntegerPattern, and Index)
+        BiConsumer<String, Consumer<Integer>> runIfValidIndexElseIndicateError =
+                (args, lambda) -> integerPattern.ifParsableElseDefault(args,
+                        integer -> taskList.ifValidIndexElse(integer,
+                                lambda,
+                                invalidIndex -> io.indicateInvalidIndex(invalidIndex, taskList.getIndexBounds())
+                        ));
+
+                //noinspection SpellCheckingInspection
+        commands.addCommand("mark",
+                        string -> runIfValidIndexElseIndicateError.accept(string,
+                                validIndex -> {
+                                    taskList.markTask(validIndex-1);
+                                    io.printTaskModified(taskList, validIndex);
+                                }
+                        )
+                )
+                .addCommand("unmark",
+                        string -> runIfValidIndexElseIndicateError.accept(string,
+                                validIndex -> {
+                                    taskList.unmarkTask(validIndex-1);
+                                    io.printTaskModified(taskList, validIndex);
+                                }
+                        )
+                )
+                .addCommand("delete",
+                        string -> runIfValidIndexElseIndicateError.accept(string,
+                                validIndex -> io.printTaskDeleted(taskList.removeTask(validIndex-1))
+                        )
+                );
+
+        // User creating new tasks (with SlashPattern)
+
+        BiConsumer<String, BiFunction<NamedParameterMap, BiConsumer<ErrorIndicatorIo.InvalidState, NamedParameterMap>, Optional<Task>>>
+                createTaskIfValidElseWarn = (args, bifunction) -> slashPattern.ifParsableElseDefault(args,
+                        namedParameterMap -> bifunction.apply(
+                                namedParameterMap,
+                                io::indicateArgumentInvalid
+                        ).ifPresent(task -> {
+                            taskList.addTask(task);
+                            io.printTaskAdded(taskList);
+                        })
+                );
+
+        commands.addCommand("todo",
+                        args -> createTaskIfValidElseWarn.accept(args, Task.Todo::createIfValidElse)
+                )
+                .addCommand("event",
+                        args -> createTaskIfValidElseWarn.accept(args, Task.Event::createIfValidElse)
+                )
+                .addCommand("deadline",
+                        args -> createTaskIfValidElseWarn.accept(args, Task.Deadline::createIfValidElse)
+                )
+        ;
+
+    }


Perhaps you could have added this to another class like CommandMap?

I actually spent quite a bit deliberating this, but I ended up not doing it (at least for now).

The part where I defend myself

My thought process was that I wanted to add bot-specific things in bot-specific classes; to change what commands are supported and what behaviours result from commands, my current iteration only requires that I change or even replace CatBot (and possibly CatBotConsoleIO) with minimal rewrite.
While there might be some merit in a CatBotCommands class that contains this method, that would leave CatBot as a small, anaemic class (I am leaning towards this being a bad thing, but I'm not too sure of myself here).
Ultimately, I had decided that it made more sense to me to have the commands be clear from the bot description, instead of its own separate class, especially since the class would otherwise be so small as to be trivial.

The part where I stop defending myself

I definitely see an alternative that takes your considerations into account. I could rename the current file to CatBotMain or CatBotEntrypoint to reflect its explicit purpose as a small and trivial class, put this initialization as a protected static method of CatBotCommandLoader, or have it be the default initialization of CatBotCommandMap.
I'd love to hear your thoughts on why this design might be preferable, though; or if you had a different design in mind, I'd like to chew on that as well.

Regardless, thanks for the feedback!

rayyan35p · 2023-09-06T16:04:10Z

src/main/java/catbot/CatBotCommandPatterns.java

+
+import java.util.function.Consumer;
+
+public class CatBotCommandPatterns {


Maybe you can consider adding header comments/javadocs? For all your other classes as well

Yup! I'll get around to it. Eventually. :')

rayyan35p · 2023-09-06T16:15:40Z

src/main/java/catbot/Parser.java

+     * @param map to store the mapping between command and argument
+     */
+    private static void parseCmdArg(String s, NamedParameterMap map) {
+        String[] cmdArg = s.split("\\s", 2);


Perhaps you can write out the variable name of cmdArg to be in full so that it is clearer?

Good catch, will do. Thanks!

rayyan35p · 2023-09-06T16:16:44Z

src/main/java/catbot/Parser.java

+        if (emptyArgument) {
+            map.addNamedParameter("", s.trim());
+        } else {
+            parseCmdArg(s, map);
+        }
+    }


I like that you still wrapped in curly brackets even though they are single statement!

remuslum

Good job 💯

nubnubyas · 2023-09-09T13:19:21Z

src/main/java/catbot/CatBot.java

+
+        // User creating new tasks (with SlashPattern)
+
+        BiConsumer<String, BiFunction<NamedParameterMap, BiConsumer<ErrorIndicatorIo.InvalidState, NamedParameterMap>, Optional<Task>>>


Perhaps u can split this line into 2? I think this line has 135 characters which exceeds the 120 characters limit stated in the coding standard.

Got it, and thanks for all the nudges for coding style.
I ended up running it through a stylechecker (finally), but I appreciate the pointers!

nubnubyas · 2023-09-09T13:47:31Z

src/main/java/catbot/io/CatbotConsoleIO.java

+        if (this.endOfCommand == null) findCommand();
+        if (this.endOfCommand == -1) return retrieve();
+        else return retrieve().substring(0, this.endOfCommand);
+    }
+
+    public String getArgs() {
+        if (this.endOfCommand == null) findCommand();
+        if (this.endOfCommand == -1) return "";
+        else return retrieve().substring(this.endOfCommand+1);


Perhaps it would be better to have the conditionals on a separate line. E.g
if (this.endOfCommand == null) {
findCommand();
}

nubnubyas · 2023-09-09T13:49:05Z

src/main/java/catbot/io/CatbotConsoleIO.java

+        if (retrieve() != null)
+            this.endOfCommand = retrieve().indexOf(" ");


It would be better to have curly brackets here based on the java coding standard.

nubnubyas

Good job for the chatbot! I really like your usage of java.util.function in your code. Just some small coding standard problems here and there. Overall, I really like your IP.

btaanish

Overall a nice attempt with a close adherence to the coding standars. Just a few nits here and there but generally well designed.

btaanish · 2023-09-09T16:10:59Z

src/main/java/catbot/CatBot.java

+                .generate(io::indicateInvalidInteger);
+        CommandPattern<NamedParameterMap> slashPattern = CatBotCommandPatterns.getSlashPatternGenerator()
+                .generate(CatBotCommandPatterns.NO_DEFAULT);
+


I like how you name the variables in a greater explanatory sense

btaanish · 2023-09-09T16:13:50Z

src/main/java/catbot/Parser.java

+        return by(delimiter, false);
+    }
+
+    public static Parser by(String delimiter, boolean emptyArgument) {


Perhaps adding javadocs for public functions might help explain their functioning in greater detail?

Noted! I'll probably get back to it at some point.

btaanish · 2023-09-09T16:14:54Z

src/main/java/catbot/CatBotCommandPatterns.java

+    //endregion
+
+    //region Slash Arguments Pattern
+    private static final SlashArgumentPatternGenerator slashPatternGenerator = new SlashArgumentPatternGenerator();


Perhaps having shorter variable names may increase the readability of the code?

Perhaps, but these are very infrequently used, so I thought it might have been better to make it very descriptive instead.
I'll think about it, but as of right now I'm still leaning towards my current convention, even if it results in excessively long variable names.

btaanish · 2023-09-09T16:16:49Z

src/main/java/catbot/io/CatbotConsoleIO.java

+        return s.replaceAll("(^|\n)", "$1"+prefix);
+    }
+
+    private void line() {


Perhaps adding a verb in the function name might be more intuitive?

Good catch; I'll do that!

btaanish · 2023-09-09T16:18:25Z

src/main/java/catbot/task/TaskList.java

+        return removed;
+    }
+
+    public void ifValidIndexElse(int index, Consumer<Integer> ifValid, Consumer<Integer> otherwise) {


Perhaps adding context to the variable names might communicate their purpose better?

This is heavily inspired by Optional's ifPresentOrElse (and other similar methods), and I'm trying to take it to its logical extensions; if, even with context, this does not seem intuitive enough, I'm happy to discuss about how to make it better!

# Conflicts: # src/main/java/catbot/CatBotCommandPatterns.java

# Conflicts: # Tasks.txt # src/main/java/catbot/CatBot.java # src/main/java/catbot/CatBotCommandPatterns.java # src/main/java/catbot/io/CatbotConsoleIO.java # src/main/java/catbot/task/Task.java # src/main/java/catbot/task/TaskList.java

# Conflicts: # Tasks.txt # src/main/java/catbot/bot/CatBot.java

# Conflicts: # src/main/java/catbot/io/CatBotConsoleIo.java # src/main/java/catbot/io/UserIo.java # src/main/java/catbot/task/TaskList.java

Mainly added javadocs for classes, and their constructors. Also included javadocs for all public functions. Private or protected functions were not thoroughly documented. Test classes were not fixed, and not updated for the new functionality.

Went in to be more thorough with code quality. I already wrote most of the things with code quality in mind, so this is not a very huge change. Added comments where I deliberately diverged from general code quality advice.

Assertions, but it's actually an extension.

# Conflicts: # src/main/java/catbot/io/CatBotConsoleIo.java

Code Quality, but I actually didn't do much

Especially the linguistic form of the starting word, which I did not pay attention to while coding.

damithc and others added 14 commits August 9, 2023 02:16

Add Gradle support

28ad2b8

Bump gradle and lib version

ed6d4d2

Feature: greet and farewell

afe47af

Feature: echo until bye

4ccdfbe

Feature: task types, task doneness, task list, and parser for slash a…

b2749c3

…rguments

Feature: textuitesting

696361d

Feature: handled various errors, implemented exceptions for missing a…

cfbfbac

…rgument errors

Feature: delete

1209ca0

Restructure project (transient)

8d970a2

Completely restructures

17a2cc2

Allow saving of todo list information through serialization

3ef9980

Merge branch 'branch-Level-7'

ac0962c

Add support for dates

e2e6f0c

Merge branch 'branch-Level-8'

09eb837

rayyan35p reviewed Sep 6, 2023

View reviewed changes

remuslum reviewed Sep 7, 2023

View reviewed changes

nubnubyas reviewed Sep 9, 2023

View reviewed changes

btaanish reviewed Sep 9, 2023

View reviewed changes

H1410101 added 11 commits September 13, 2023 09:47

Rename for clarity and convention

021f2e1

Merge remote-tracking branch 'origin/add-gradle-support'

26f5425

Verify Gradle and JAR

5218357

Add JUnit tests and fix minor bugs

84540d8

Add JavaDoc to half of the classes, especially Interfaces

347cb97

Fix all coding standard except JavaDoc

76f3604

Add find functionality

13fa8fb

Merge branch 'branch-Level-9'

493a117

Merge branch 'branch-A-JavaDoc'

50839b0

# Conflicts: # src/main/java/catbot/CatBotCommandPatterns.java

Merge branch 'branch-A-CodingStandard'

901f2c7

# Conflicts: # Tasks.txt # src/main/java/catbot/CatBot.java # src/main/java/catbot/CatBotCommandPatterns.java # src/main/java/catbot/io/CatbotConsoleIO.java # src/main/java/catbot/task/Task.java # src/main/java/catbot/task/TaskList.java

Refactor for better OOP and style

bb8638f

H1410101 and others added 21 commits September 15, 2023 11:12

Refactored for OOP, fixed indexing bug

a40bfab

Add working JavaFX prototype, expecting refactoring and reorganisation

d305a5a

Add working JavaFX UI for chatbot

3fd5f4e

Merge branch 'branch-A-Varargs'

3c32b3c

# Conflicts: # Tasks.txt # src/main/java/catbot/bot/CatBot.java

Merge branch 'branch-A-CheckStyle'

66ff7aa

# Conflicts: # src/main/java/catbot/io/CatBotConsoleIo.java # src/main/java/catbot/io/UserIo.java # src/main/java/catbot/task/TaskList.java

Add javafxplugin to build.gradle

f91fbe1

Remove irrelevant todos and clean up warnings

7b0e464

Add update/edit functionality

6ceecdd

Fix style for checkstyle

77825cb

Mainly added javadocs for classes, and their constructors. Also included javadocs for all public functions. Private or protected functions were not thoroughly documented. Test classes were not fixed, and not updated for the new functionality.

Fix non-checkstyle code quality

f10b024

Went in to be more thorough with code quality. I already wrote most of the things with code quality in mind, so this is not a very huge change. Added comments where I deliberately diverged from general code quality advice.

Merge pull request #2 from H1410101/branch-A-Assertions

e351a0f

Assertions, but it's actually an extension.

Merge branch 'master' into branch-A-CodeQuality

6e2d091

# Conflicts: # src/main/java/catbot/io/CatBotConsoleIo.java

Merge pull request #3

0a08fda

Code Quality, but I actually didn't do much

Make header comments adhere to conventions

7e1f6f6

Especially the linguistic form of the starting word, which I did not pay attention to while coding.

Update GUI, implemented delayed application exit

030f583

Add UI screenshot, add acknowledgement when task list is empty

cb5ae6c

Update README.md

f2798ae

Added acknowledgement when unnumbered lists are empty

9a5badd

Merge remote-tracking branch 'origin/master'

976c574

Polish UI, including rescaling images, changing padding, etc

fa5cdf8

Format README.md

4893ea7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[H1410101] iP #555

[H1410101] iP #555

H1410101 commented Sep 4, 2023 •

edited

Loading

rayyan35p left a comment

rayyan35p Sep 6, 2023

H1410101 Sep 8, 2023

rayyan35p Sep 6, 2023

H1410101 Sep 8, 2023

rayyan35p Sep 6, 2023

H1410101 Sep 8, 2023

rayyan35p Sep 6, 2023

remuslum left a comment

nubnubyas Sep 9, 2023

H1410101 Sep 13, 2023

nubnubyas Sep 9, 2023

nubnubyas Sep 9, 2023

nubnubyas left a comment

btaanish left a comment

btaanish Sep 9, 2023

btaanish Sep 9, 2023

H1410101 Sep 13, 2023

btaanish Sep 9, 2023

H1410101 Sep 13, 2023

btaanish Sep 9, 2023

H1410101 Sep 13, 2023

btaanish Sep 9, 2023

H1410101 Sep 13, 2023


		import java.util.function.Consumer;

		public class CatBotCommandPatterns {


		// User creating new tasks (with SlashPattern)

		BiConsumer<String, BiFunction<NamedParameterMap, BiConsumer<ErrorIndicatorIo.InvalidState, NamedParameterMap>, Optional<Task>>>

		if (retrieve() != null)
		this.endOfCommand = retrieve().indexOf(" ");

[H1410101] iP #555

Are you sure you want to change the base?

[H1410101] iP #555

Conversation

H1410101 commented Sep 4, 2023 • edited Loading

CatBot

rayyan35p left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The part where I defend myself

The part where I stop defending myself

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remuslum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nubnubyas left a comment

Choose a reason for hiding this comment

btaanish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

H1410101 commented Sep 4, 2023 •

edited

Loading